Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Move storybook.requires.js and Storybook.tsx files from the root directory to the .storybook folder #252

Merged

Conversation

lauriharpf
Copy link
Contributor

Issue: #238

What I did

Changed the loader.js script (that generates storybook.requires.js), the V6 alpha instructions and the example project so that both storybook.requires.js and Storybook.tsx are moved to the .storybook directory from the root.

How to test

  1. Use React Native example project in the /examples/native directory. Confirm that the stories still work both on iOS and Android.
  2. Run yarn prepareAll in the root of the project. Then follow the v6 alpha setup guide in v6README.md, but in the same way as in fix: Fix #120 (running "yarn web" on Expo fails). #251 , use the local Storybook React Native dependencies instead of the ones pulled from NPM.

Note: In step 2, cd ios; pod install; cd ..; fails on my box with the following error:

[!] Invalid `Podfile` file: cannot load such file -- /Users/lauriharpf/Repos/lauriharpf/RnSBSixAlpha/node_modules/react-native/scripts/react_native_pods.

 #  from /Users/lauriharpf/Repos/lauriharpf/RnSBSixAlpha/ios/Podfile:1
 #  -------------------------------------------
 >  require_relative '../node_modules/react-native/scripts/react_native_pods'
 #  require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'
 #  -------------------------------------------

Running yarn in the root of RnSBSixAlpha fixes it. I'm unsure why this happens, but I suspect that it might be caused by using the local React Native Storybook dependencies. The same thing happens with the next-6.0 branch, so it shouldn't be related to this change.

  • Does this need a new example in examples/native? no, not in my opinion
  • Does this need an update to the documentation? no, not in my opinion

…k.tsx files from the root directory to the .storybook folder.
@lauriharpf lauriharpf requested a review from dannyhw as a code owner August 14, 2021 16:17
@dannyhw
Copy link
Member

dannyhw commented Aug 14, 2021

Are you able to test this with expo? I had some issues with the .storybook folder in expo because of the "." it seemed like metro didn't want to find hidden files.

@lauriharpf
Copy link
Contributor Author

Are you able to test this with expo? I had some issues with the .storybook folder in expo because of the "." it seemed like metro didn't want to find hidden files.

Good observation! Tested this now with Expo. It seems to work on my machine, but for some reason requires running yarn before yarn ios or yarn android. If I don't do that, then I get this error:

lauriharpf@Lauris-MacBook-Pro appName % yarn ios
yarn run v1.22.10
$ expo start --ios
Starting project at /Users/lauriharpf/Repos/lauriharpf/appName
Developer tools running on http://localhost:19002
Opening developer tools in the browser...
Missing package "metro" in the project at /Users/lauriharpf/Repos/lauriharpf/appName. This usually means `react-native` is not installed. Please verify that dependencies in package.json include "react-native" and run `yarn` or `npm install`.
Error: Missing package "metro" in the project at /Users/lauriharpf/Repos/lauriharpf/appName. This usually means `react-native` is not installed. Please verify that dependencies in package.json include "react-native" and run `yarn` or `npm install`.
    at importMetroFromProject (/Users/lauriharpf/.nvm/versions/node/v16.4.0/lib/node_modules/expo-cli/node_modules/@expo/dev-server/src/MetroDevServer.ts:236:11)
    at runMetroDevServerAsync (/Users/lauriharpf/.nvm/versions/node/v16.4.0/lib/node_modules/expo-cli/node_modules/@expo/dev-server/src/MetroDevServer.ts:61:17)
    at startDevServerAsync (/Users/lauriharpf/.nvm/versions/node/v16.4.0/lib/node_modules/expo-cli/node_modules/xdl/src/start/startDevServerAsync.ts:62:55)
    at startAsync (/Users/lauriharpf/.nvm/versions/node/v16.4.0/lib/node_modules/expo-cli/node_modules/xdl/src/start/startAsync.ts:72:41)

As mentioned in the PR description, this could be related to pointing to the local NPM packages, but I'm not sure. Here are the steps that I did to experiment with this branch on Expo (replace /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/with your React Native Storybook repository location):

git checkout feat/move-files-to-storybook-dir
yarn prepareAll
cd ..

npm install --global expo-cli
expo init -t expo-template-blank-typescript expotest
cd expotest

yarn add /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/app/react-native \
            @react-native-async-storage/async-storage \
            /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/addons/ondevice-actions \
            /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/addons/ondevice-controls  \
            /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/addons/ondevice-backgrounds \
            /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/addons/ondevice-notes  \
            @storybook/addon-actions \
            @react-native-community/datetimepicker \
            @react-native-community/slider \
            @storybook/addon-controls

yarn

echo "const { getDefaultConfig } = require('expo/metro-config');
const defaultConfig = getDefaultConfig(__dirname);
defaultConfig.resolver.resolverMainFields = [
  'sbmodern',
  ...defaultConfig.resolver.resolverMainFields,
];
defaultConfig.transformer.getTransformOptions = async () => ({
  transform: {
    experimentalImportSupport: false,
    inlineRequires: false,
  },
});
module.exports = defaultConfig;
" > metro.config.js;

mkdir .storybook
mkdir components
echo "module.exports = {
  stories: [
    './components/**/*.stories.?(ts|tsx|js|jsx)'
  ],
   addons: [
    '@storybook/addon-ondevice-notes',
    '@storybook/addon-ondevice-controls',
    '@storybook/addon-ondevice-backgrounds',
    '@storybook/addon-ondevice-actions',
  ],
};" > .storybook/main.js

echo "import {withBackgrounds} from '@storybook/addon-ondevice-backgrounds';

export const decorators = [withBackgrounds];
export const parameters = {
  backgrounds: [
    {name: 'plain', value: 'white', default: true},
    {name: 'warm', value: 'hotpink'},
    {name: 'cool', value: 'deepskyblue'},
  ],
};" > .storybook/preview.js

echo "import AsyncStorage from '@react-native-async-storage/async-storage';
import { getStorybookUI } from '@storybook/react-native';

import './storybook.requires';

const StorybookUIRoot = getStorybookUI({
  asyncStorage: AsyncStorage,
});

export default StorybookUIRoot;" >.storybook/Storybook.tsx

echo "import StorybookUIRoot from './.storybook/Storybook';
export { StorybookUIRoot as default };" > App.tsx

node -e 'const fs = require("fs");
const packageJSON = require("./package.json");
packageJSON.scripts = {
    ...packageJSON.scripts,
    prestart: "sbn-get-stories",
    "storybook-watcher": "sbn-watcher"
};
fs.writeFile("./package.json", JSON.stringify(packageJSON, null, 2), function writeJSON(err) {
  if (err) return console.log(err);
  console.log(JSON.stringify(packageJSON));
  console.log("writing to " + "./package.json");
});';

mkdir components/Button;
echo "import React from 'react';
import {TouchableOpacity, Text, StyleSheet} from 'react-native';

interface MyButtonProps {
  onPress: () => void;
  text: string;
}

export const MyButton = ({onPress, text}: MyButtonProps) => {
  return (
    <TouchableOpacity style={styles.container} onPress={onPress}>
      <Text style={styles.text}>{text}</Text>
    </TouchableOpacity>
  );
};

const styles = StyleSheet.create({
  container: {
    paddingHorizontal: 16,
    paddingVertical: 8,
    backgroundColor: 'violet',
  },
  text: {color: 'black'},
});
" > components/Button/Button.tsx;

echo "import React from 'react';
import {ComponentStory, ComponentMeta} from '@storybook/react-native';
import {MyButton} from './Button';

const MyButtonMeta: ComponentMeta<typeof MyButton> = {
  title: 'MyButton',
  component: MyButton,
  argTypes: {
    onPress: {action: 'pressed the button'},
  },
  args: {
    text: 'Hello world',
  },
};

export default MyButtonMeta;

type MyButtonStory = ComponentStory<typeof MyButton>;

export const Basic: MyButtonStory = args => <MyButton {...args} />;

" > components/Button/Button.stories.tsx

yarn sbn-get-stories

Now yarn ios and yarn android should work. While testing, I noticed that yarn needs to be run right after adding the yarn add for storybook dependencies. If I run yarn after yarn sbn-get-stories, then I get an error about the Storybook.tsx not being found.

@dannyhw
Copy link
Member

dannyhw commented Aug 15, 2021

If I run yarn after yarn sbn-get-stories, then I get an error about the Storybook.tsx not being found.

@lauriharpf that's a really strange behaviour 🤔. If the .storybook folder is renamed without the "." does it work more consistently?

@lauriharpf
Copy link
Contributor Author

If I run yarn after yarn sbn-get-stories, then I get an error about the Storybook.tsx not being found.

@lauriharpf that's a really strange behaviour 🤔. If the .storybook folder is renamed without the "." does it work more consistently?

Yeah, it's puzzling to me as well. Good suggestion, haven't tested that yet - will try to find time for it e.g. tomorrow. Could also double-check how the next-6.0 branch works in comparison to this branch.

@lauriharpf
Copy link
Contributor Author

@lauriharpf that's a really strange behaviour 🤔. If the .storybook folder is renamed without the "." does it work more consistently?

Yeah, it's puzzling to me as well. Good suggestion, haven't tested that yet - will try to find time for it e.g. tomorrow. Could also double-check how the next-6.0 branch works in comparison to this branch.

Was now able to test this in more detail:

  1. Using this branch, but renaming the .storybook folder to storybook. Slightly adjusted the code and then did this: https://gist.github.com/lauriharpf/3a9efb9150880cadaa87c52dbf8d9aba
  2. Using the next-6.0 branch as-is, https://gist.github.com/lauriharpf/eeafd6356678f4d46f663e049c4784c5
  3. Using the released 6.0.1 alpha (same as above, but pointing to the dependencies in NPM instead)
  4. Using the commit the 6.0.1 alpha was released from (2bcd2fd) and pointing the yarn dependencies to the local directories

All of the experiments where I had local dependencies, or in other words did this

yarn add /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/app/react-native \
            @react-native-async-storage/async-storage \
            /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/addons/ondevice-actions \
            /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/addons/ondevice-controls  \
            /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/addons/ondevice-backgrounds \
            /Users/lauriharpf/Repos/lauriharpf/react-native-storybook/addons/ondevice-notes  \
            @storybook/addon-actions \
            @react-native-community/datetimepicker \
            @react-native-community/slider \
            @storybook/addon-controls

caused yarn ios to fail with

yarn run v1.22.10
$ expo start --ios
Starting project at /Users/lauriharpf/Repos/lauriharpf/expotest
Developer tools running on http://localhost:19002
Opening developer tools in the browser...
Missing package "metro" in the project at /Users/lauriharpf/Repos/lauriharpf/expotest. This usually means `react-native` is not installed. Please verify that dependencies in package.json include "react-native" and run `yarn` or `npm install`.
Error: Missing package "metro" in the project at /Users/lauriharpf/Repos/lauriharpf/expotest. This usually means `react-native` is not installed. Please verify that dependencies in package.json include "react-native" and run `yarn` or `npm install`.
    at importMetroFromProject (/Users/lauriharpf/.nvm/versions/node/v16.4.0/lib/node_modules/expo-cli/node_modules/@expo/dev-server/src/MetroDevServer.ts:236:11)
    at runMetroDevServerAsync (/Users/lauriharpf/.nvm/versions/node/v16.4.0/lib/node_modules/expo-cli/node_modules/@expo/dev-server/src/MetroDevServer.ts:61:17)
    at startDevServerAsync (/Users/lauriharpf/.nvm/versions/node/v16.4.0/lib/node_modules/expo-cli/node_modules/xdl/src/start/startDevServerAsync.ts:62:55)
    at startAsync (/Users/lauriharpf/.nvm/versions/node/v16.4.0/lib/node_modules/expo-cli/node_modules/xdl/src/start/startAsync.ts:72:41)
error Command failed with exit code 1.

unless I ran yarn after the yarn add. Pointing to the NPM dependencies instead of my local directories worked without the yarn.

Since the behaviour was the same for commit 2bcd2fda528593b33decb261a32faf229a215ad8 which should be identical to the released NPM packages, I'm guessing that this weird behaviour is a side-effect of using local folders as yarn dependencies. Don't know what exactly is causing it, but it doesn't seem to be related to this PR.

Copy link
Member

@dannyhw dannyhw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking at this again now and it seems fine to me, I think the best course of action would be to merge and then test it in the next alpha. That way we can be sure the issues aren't some edge case of using local dependencies.

Sorry for the delay, taken me a bit to get back into the swing of things after my holiday 😅 .

@dannyhw dannyhw merged commit 57d7317 into storybookjs:next-6.0 Sep 9, 2021
@lauriharpf lauriharpf deleted the feat/move-files-to-storybook-dir branch September 11, 2021 19:26
dannyhw pushed a commit to raychanks/react-native that referenced this pull request Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants